Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix two-phase delete buttons with Safari #382

Closed
wants to merge 1 commit into from
Closed

Fix two-phase delete buttons with Safari #382

wants to merge 1 commit into from

Conversation

JasonWeill
Copy link
Collaborator

Fixes #364 for Safari (desktop macOS) compatibility by removing the onBlur event on the ConfirmButton component. We had presumed that, after clicking the initial X icon, the focus would move to the Button component that did the actual delete, but in Safari, this is not the case.

As a noticeable UI change, this means that it's possible to have more than one red "Delete" button visible at a time. It is also no longer possible to cancel a two-phase Delete without reloading the view or the page, but I expect that this is a rare use case.

delete-buttons

@JasonWeill JasonWeill added the bug Something isn't working label Jun 10, 2023
@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch JasonWeill/jupyter-scheduler/safari-delete

@3coins
Copy link
Collaborator

3coins commented Jun 10, 2023 •
edited
Loading

@JasonWeill
Thanks for working on this. This fixes the issue in Safari, but causes a regression. The Delete button doesn't reset (hide) when you click outside the button. Reloading the view doesn't change the state of the button.

Rather than trying to fix this in Safari, I think we should re-think the overall selection/delete UX, and work on allowing multi-selection/delete which will solve this problem as well.

@JasonWeill
Copy link
Collaborator Author

Today I learned that "Clicking on a non-text input element does not give it focus" in WebKit (Safari) is a WONTFIX bug in WebKit, opened in 2008, resolved a long time ago, and still attracting the old comment or duplicate bug.

The regression is deliberate for the sake of Safari compatibility; we might be able to avoid it if we can coerce Safari into focusing the Button when we make it visible.

This was referenced Jun 12, 2023
Closed
Merged
@JasonWeill
Copy link
Collaborator Author

Closing in favor of #385.

@JasonWeill JasonWeill closed this Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot delete jobs in Safari browser
2 participants
@JasonWeill @3coins